fix: propagate json.Unmarshal error in mysql rowDataConverter#6721
fix: propagate json.Unmarshal error in mysql rowDataConverter#6721vedant21-oss wants to merge 1 commit intopipe-cd:masterfrom
Conversation
vedant21-oss
force-pushed
the
fix-mysql-iterator-json-error
branch
from
9fdb89b to
28a2188
Compare
eeshaanSA
left a comment
eeshaanSA
left a comment
There was a problem hiding this comment.
Hey @vedant21-oss, thanks for this. Do you have any logs that can support this?
Did anyone try reproducing?
vedant21-oss
force-pushed
the
fix-mysql-iterator-json-error
branch
from
28a2188 to
b8199ef
Compare
|
Hey @eeshaanSA, thanks for the review! I just updated the PR to include a unit test that explicitly reproduces this behavior in Without the fix, the JSON unmarshaling silently failed, and an empty/partial object was returned. With this fix, running the tests explicitly captures the error. Here is the log output from the new testcase simulating invalid JSON data from the database:
The unmarshal error is now properly propagated up to the caller instead of being swallowed. I've force-pushed the test coverage alongside the fix. |
rahulshendre
left a comment
rahulshendre
left a comment
There was a problem hiding this comment.
Reproduced locally. All tests pass, invalid_json_data catches the error correctly.
One note:
Cursor() error path (lines 62-65 of iterator.go) isn't covered by any test case, it returns nil.
@vedant21-oss i think we should add a case there too
Signed-off-by: vedant21-oss <vedantmalodeofficial@gmail.com>
vedant21-oss
force-pushed
the
fix-mysql-iterator-json-error
branch
from
b8199ef to
38143bd
Compare
|
hey @vedant21-oss, any updates on this one 👀 |
3 participants
Resolves #6699
The rowDataConverter in the MySQL datastore now returns an error that bubbles up through Iterator.Next() to handle silent json.Unmarshal failures.